Skip to content

Optimize model building pipeline: defer Dependency.build() and reduce allocations - #12653

Draft
gnodet wants to merge 4 commits into
masterfrom
optimize-model-building-pipeline-defer-dependency
Draft

Optimize model building pipeline: defer Dependency.build() and reduce allocations#12653
gnodet wants to merge 4 commits into
masterfrom
optimize-model-building-pipeline-defer-dependency

Conversation

@gnodet

@gnodet gnodet commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add Builder getters to generated model classes (model.vm) — enables reading builder fields through the base chain without calling build(), supporting deferred materialization across pipeline stages
  • Add *ToBuilder merger variants (merger.vm) — returns Builder instead of calling build(), letting callers accumulate changes across multiple merge passes without intermediate allocations
  • Defer build() in DefaultDependencyManagementInjector — accumulates merged dependencies as Builder objects, calling build() only once per modified dependency at the end of the loop
  • Optimize computeLocations() — replaces Stream.concat().collect(Collectors.toUnmodifiableMap(...)) with HashMap.putAll() + Map.copyOf(), and returns oldlocs directly when newlocs is empty (avoids unnecessary Map.copyOf since the base map is already immutable)
  • Precompute locationsHashCode — cached at build time in the generated constructor, used by DefaultModelObjectPool.PoolKey for fast inequality checks before full map comparison
  • Optimize PoolKey.locationsEqual() — uses direct getLocations() map comparison instead of iterating individual keys, with fast-path for both-empty maps (common for imported deps)
  • Add addLocationInformation API to XmlReaderRequest — wired through DefaultModelXmlFactory for future use in skipping location tracking on imported BOM POMs

Context

JFR profiling on a 4,383-module reactor shows 37% CPU in ModelObjectPool (dependency interning), 18% in Dependency immutable builders, and 2.3 GB allocated in KeyValueHolder from computeLocations(). Each Dependency passes through ~7 pipeline stages, each calling build() which allocates a new immutable object + recomputes location maps + calls processObject().

This PR targets the three hottest code paths:

  1. Redundant build() calls — deferred via Builder getters and ToBuilder merger variants
  2. computeLocations() stream overhead — replaced with HashMap merge + early return
  3. PoolKey location comparison — precomputed hash + direct map equality

Test plan

  • mvn verify -B passes across all reactor modules (all tests green)
  • FileToRawModelMergerTest updated to exclude new *ToBuilder methods from reflection-based override check
  • Benchmark with JFR on large reactor to measure allocation reduction

🤖 Generated with Claude Code

gnodet and others added 3 commits July 31, 2026 23:52
…mance

JFR profiling of a 4,383-module reactor revealed three hotspots that
together consume ~35% of CPU time during dependency resolution:

1. DefaultGraphBuilder: result.sort(comparing(sortedProjects::indexOf))
   uses O(n) ArrayList.indexOf per comparison, causing O(n² log n) total
   MavenProject.equals calls (~230M for 4,383 modules). Replace with a
   HashMap<MavenProject, Integer> index built in O(n), reducing sort
   comparisons to O(1) each. Affects 3 call sites.

2. DefaultModelObjectPool.getPooledTypes(): re-parses a comma-separated
   property string into a new Stream → Set on every process() call.
   Cache the parsed Set at construction time. Also add hashCode
   fast-rejection to PoolKey.equals() to skip expensive deep equality
   when hashes differ.

3. DefaultModelObjectPool.PoolKey.dependencyHashCode(): Objects.hash()
   with 12 arguments allocates a new Object[12] on every call. Inline
   the hash computation to eliminate the varargs allocation.

4. PhaseComparator: List.indexOf() in compare() is O(n) per call.
   Pre-build a HashMap<String, Integer> in the constructor for O(1)
   phase index lookups.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… allocations

Reduce CPU and memory overhead in Maven 4's immutable model building
pipeline by deferring Dependency.build() across pipeline stages and
optimizing hot paths in model object pooling.

Key changes:
- Add Builder getters to generated model classes (model.vm) enabling
  field access without materializing immutable objects
- Add *ToBuilder merger variants (merger.vm) that return Builder instead
  of calling build(), letting callers accumulate changes across stages
- Defer build() in DependencyManagementInjector to batch-build only
  modified dependencies at the end of the merge loop
- Replace Stream.concat().collect() with HashMap.putAll() in
  computeLocations() and precompute locations hash code to eliminate
  repeated map iteration during pooling
- Optimize PoolKey.locationsEqual() to use direct map comparison with
  fast-path for empty maps and hash-based inequality check
- Add addLocationInformation API to XmlReaderRequest for future use
  in skipping location tracking on imported BOMs

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…r through pipeline

Extend the model code generation (model.vm) so that Builder classes store
Collection<X.Builder> instead of Collection<X> for model-class list fields.
This enables accumulating changes across pipeline stages without intermediate
build() calls.

Key changes:
- model.vm: Builder fields for model-class lists now use child builders.
  Backward-compatible setter wraps immutable objects into builders.
  Added getModifiable*() methods for lazy base-list wrapping.
  Added reset(T base) method to replace builder state in-place.
  Short-circuit optimization: skip build when no fields are set.

- Pipeline stage interfaces (10 interfaces): added default builder-accepting
  methods that bridge to the existing Model-accepting implementations.
  Fully backward compatible for existing implementations.

- DefaultModelBuilder: buildEffectiveModel() and readEffectiveModel() now
  thread a Model.Builder between stages instead of rebuilding at each step.

- Hot stage implementations: overrode builder-accepting methods in
  DefaultModelNormalizer, DefaultDependencyManagementInjector,
  DefaultPluginManagementInjector, DefaultModelPathTranslator, and
  DefaultPluginConfigurationExpander to write directly to the passed builder,
  avoiding redundant newBuilder() allocations and intermediate build() calls.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet
gnodet force-pushed the optimize-model-building-pipeline-defer-dependency branch from 46712b6 to 5e74cd4 Compare August 1, 2026 05:57
Add getBuilt*() methods to Builder for model-object list fields that
return List<X> by resolving through base when unmodified (zero cost)
or building just that field's builders (avoids full model materialization).

Fix 5 pipeline stages to use builder getters instead of builder.build():
- DefaultPluginConfigurationExpander: getBuild()/getReporting()
- DefaultModelNormalizer: getBuild()/getBuiltDependencies()
- DefaultDependencyManagementInjector: getDependencyManagement()/getBuiltDependencies()
- DefaultPluginManagementInjector: getBuild()
- DefaultModelPathTranslator: getBuild()/getReporting()

Each stage previously called builder.build() to read 1-2 fields, which
triggered full model materialization including wrap/unwrap of all
model-object list fields. With builder getters, only the needed fields
are accessed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet

gnodet commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Benchmark Results (4383-module project, mvn validate)

Added commit 568b44b that eliminates unnecessary builder.build() calls in pipeline stages by using targeted builder getters instead of full model materialization.

Changes

model.vm: Added getBuilt*() methods for model-object list fields. When the builder field is null (unmodified), returns base.getXxx() at zero cost. When modified, builds just that field's builders — avoids full model build.

5 pipeline stages fixed to avoid builder.build():

  • DefaultPluginConfigurationExpanderbuilder.getBuild() / builder.getReporting()
  • DefaultModelNormalizer (2 methods) → builder.getBuild() / builder.getBuiltDependencies()
  • DefaultDependencyManagementInjectorbuilder.getDependencyManagement() / builder.getBuiltDependencies()
  • DefaultPluginManagementInjectorbuilder.getBuild()
  • DefaultModelPathTranslatorbuilder.getBuild() / builder.getReporting()

Results (3 runs each, average)

Version Average vs rc-6
rc-6 baseline 22,114ms
PR #12652 (pool+sort) 18,617ms -15.8%
PR #12653 (before this commit) 17,739ms -19.8%
PR #12653 (with this commit) 17,956ms -18.8%

Performance is within run-to-run variance. The fix eliminates the unnecessary full-model materializations while maintaining the same performance level. All 1130 tests pass (550 in maven-impl, 580 in maven-core).

@gnodet

gnodet commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Updated benchmark: getModifiable() + two-field optimization

Following up on the previous fix — the pipeline stages now use getModifiable*() to work directly with mutable Dependency.Builder lists, mutating in place instead of round-tripping through immutable objects.

What changed (commit 443dcc5)

model.vm (two-field optimization):

  • Builder now stores both Collection<X.Builder> (for getModifiable*()) and Collection<X> Imm (for the setter)
  • The dependencies(Collection<Dependency>) setter stores the immutable list directly — zero wrapping overhead
  • getModifiable*() lazily wraps from whichever source is set (immutable field, or base)
  • build() constructor uses whichever field is set — immutable list path has no builder overhead
  • forceCopy constructor stores immutable reference instead of wrapping

Pipeline stages (getModifiable pattern):

  • DefaultModelNormalizer.injectDefaultValues — iterates getModifiableDependencies(), sets scope on builders in place
  • DefaultModelNormalizer.mergeDuplicates — deduplicates builder list in place using management key computed from builder getters
  • DefaultDependencyManagementInjector — new mergeManagementIntoBuilders() method merges managed dep fields (version, scope, type, classifier, systemPath, exclusions + locations) directly into Dependency.Builder objects

Cost model

Before (v1): each pipeline stage that touches deps does a full wrap + unwrap cycle:

builder.getBuiltDependencies()  → N unwrap allocations
builder.dependencies(newList)   → N wrap allocations

With 3 stages touching deps: 6N allocations per module (3 wraps + 3 unwraps)

After (v2): ONE lazy wrap + ONE final unwrap for the entire pipeline segment:

builder.getModifiableDependencies()  → lazy wrap (cached across stages)
  stage 1: modify builders in place   → 0 allocations
  stage 2: modify builders in place   → 0 allocations
  stage 3: modify builders in place   → 0 allocations
builder.build()                      → final unwrap

Total: 2N allocations per module (1 wrap + 1 unwrap)

Benchmark results (4383-module project, mvn validate --threads 1)

Build Average vs rc-6 vs PR #12652
rc-6 baseline 20,234ms
PR #12652 (pool+sort) 14,936ms -26.2%
PR #12653 v1 (getBuilt) 14,784ms -26.9% -1.0%
PR #12653 v2 (getModifiable) 13,385ms -33.8% -10.4%

The v2 optimization delivers a clear 10% improvement over PR #12652 alone, and 34% over rc-6.

Branch: optimize-model-builder-pipeline-fix (commits 568b44b861 + 443dcc5b51)

@gnodet

gnodet commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

JFR-guided optimization (v3): withImportedFrom + ConcurrentHashMap.newKeySet

JFR profiling on the 4383-module benchmark identified two remaining hotspots, now fixed in commit d30efb5:

CPU profile comparison (v2 → v3)

Component v2 samples v2 % v3 samples v3 % Change
DependencyMgmtImporter 442 43.0% 143 20.4% -68%
updateWithImportedFrom 354 34.4% 44 6.3% -88%
CopyOnWriteArraySet 68 6.6% 0 0.0% -100%
ModelValidator 71 6.9% 66 9.4% same
I/O (XML, Zip, Jar) 78 7.6% 86 12.3% same
Other 369 35.9% 406 57.9% same
Total CPU samples 1028 701 -32%

What was fixed

1. updateWithImportedFrom (was 35% of CPU)

Every BOM-imported dependency was rebuilt through Dependency.newBuilder(dep, true).importedFrom(loc).build() — triggering forceCopy (wraps all sub-lists into builders), build (unwraps back to immutable), and object pool interning — just to set one metadata field.

Added withImportedFrom(InputLocation) + private copy constructor to model.vm. Shares all model fields by reference, bypasses Builder and object pool entirely.

2. CopyOnWriteArraySet (was 6.6% of CPU)

ConsumerPomArtifactTransformer.deferDeleteFile() added to a CopyOnWriteArraySet<Path> → O(n) indexOfRange scan + array copy per add. With 4383 modules: O(n²). Replaced with ConcurrentHashMap.newKeySet() → O(1).

Wall-clock benchmark (4383 modules, mvn validate)

Wall-clock difference between v2 and v3 is within noise (~0.3s on a 14s benchmark) because:

  • BOM import runs once on the root POM, not on the critical path for parallel child processing
  • The parallel 4383-module build is I/O and scheduling dominated

The cumulative improvement from rc-6 baseline remains ~27% faster.

@gnodet
gnodet requested a review from cstamas August 1, 2026 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant